FIX+ENH: Time conversions for CSV parser and HTML table parser #2329
+263
−77
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gethvi
force-pushed
the
fix-csv-parser
branch
2 times, most recently
from
d3ff5ab
to
3c2d5f6
Compare
gethvi
force-pushed
the
fix-csv-parser
branch
12 times, most recently
from
0ad0e84
to
208743b
Compare
gethvi
changed the title
|
Ready for review. |
gethvi
force-pushed
the
fix-csv-parser
branch
4 times, most recently
from
0c0c837
to
d841782
Compare
sebix
reviewed
Mar 16, 2023
gethvi
force-pushed
the
fix-csv-parser
branch
2 times, most recently
from
fc197fa
to
9f14b68
Compare
kamil-certat
reviewed
Apr 6, 2023
gethvi
force-pushed
the
fix-csv-parser
branch
2 times, most recently
from
b7c8119
to
11fd27f
Compare
gethvi
force-pushed
the
fix-csv-parser
branch
2 times, most recently
from
7e9c755
to
b940af5
Compare
kamil-certat
approved these changes
May 17, 2023
* Adds new `TimeFormat` class for `time_format` bot parameter. It improves performance, as it validates the parameter only once on instantiation of the bot class and not every time datetime is parsed (looking at you HTML Table parser). Also removes some code duplicity. * Changes CSV Parser time conversions. For some reason the CSV parser had it's own `TIME_CONVERSIONS` and it was very limited. This PR changes it to use `DateTime.TIME_CONVERSIONS`. Now CSV parser uses `TimeFormat` class for `time_format` parameter. * Changes HTML Table parser to use `TimeFormat` class for `time_format` parameter as well. * Changes `DateTime` conversion function names to consistent naming scheme starting with `from_`. Changes function signature to be consistent. Backwards compatible. * Updates some docstrings. * Fixes a bug in `InvalidArgument` exception.
This was referenced May 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR:
TimeFormatclass fortime_formatbot parameter. It improves performance, as it validates the parameter only once on instantiation of the bot class and not every time datetime is parsed (looking at you HTML Table parser). Also removes some code duplicity.TIME_CONVERSIONSand it was very limited (e.g.from_formatcouldn't be used, see Having issues with intelmq.bots.collectors.file.collector_file #2326). This PR changes it to useDateTime.TIME_CONVERSIONS. Now CSV parser usesTimeFormatclass fortime_formatparameter.TimeFormatclass fortime_formatparameter as well.DateTimeconversion function names to consistent naming scheme starting withfrom_. Changes function signature to be consistent. Backwards compatible.InvalidArgumentexception.